Add Windows terminal backend support #1012

SmartManoj · 2025-11-04T08:44:10Z

Introduces WindowsTerminal for PowerShell-based command execution on Windows systems. Updates factory and init logic to conditionally import and use platform-specific terminal backends, ensuring compatibility across Windows and Unix-like platforms.

Introduces WindowsTerminal for PowerShell-based command execution on Windows systems. Updates factory and init logic to conditionally import and use platform-specific terminal backends, ensuring compatibility across Windows and Unix-like platforms.

xingyaoww

I wonder if we should rename the tool execute_bash into execute_terminal because of the powershell here? 🤔

And we tell the agent if it is using powershell vs bash. But we can do this in subsequent pr

Simplifies the code by unconditionally importing fcntl and pty, removing the platform-specific logic and dummy assignments for Windows. This module is intended for Unix-like systems only.

SmartManoj · 2025-11-05T08:27:47Z

Deleted scripts/demo.py as it is no longer needed. The demo script previously demonstrated basic usage of the OpenHands SDK and agent setup.

Eliminated an erroneous 'n' character from the import block in terminal_session.py to clean up the code and prevent potential syntax errors.

Changed assertions in test_windows_terminal_close_and_reopen to check 'obs.text' instead of 'obs.output' for command results. This aligns the test with the updated observation object structure.

xingyaoww · 2025-11-06T19:02:16Z

@SmartManoj can you resolve merge conflicts/

Update the terminal module's __init__.py to conditionally import WindowsTerminal on Windows and SubprocessTerminal/TmuxTerminal on other platforms. This change ensures only relevant terminal classes are exposed based on the operating system.

Removed the deprecated execute_bash.terminal package and updated all relevant imports to use the new openhands.tools.terminal.terminal structure. This change consolidates terminal-related code under a single module for improved maintainability and clarity.

blacksmith-sh · 2025-11-10T12:59:39Z

[Automatic Post]: This PR seems to be currently waiting for review. @ryanhoangt, could you please take a look when you have a chance?

ryanhoangt

Thank you for the PR. When testing the PR on the examples/01_hello_world.py and adjusting the message to something like "Please check the current directory, list all files and then exit.", I saw it first attempted to run a bash command with &&, which failed in PowerShell. However, it didn't fail instantly, it got stuck in soft timeout observation loop and the agent decided to reset the terminal. Do you have any ideas why it happened?

ryanhoangt · 2025-11-17T10:38:49Z

openhands-tools/openhands/tools/terminal/terminal/__init__.py

-]
+# Conditionally import platform-specific terminals
+if platform.system() == "Windows":
+    from openhands.tools.terminal.terminal.windows_terminal import WindowsTerminal


Is there any reason why we export things conditionally like this? I think it might break several test files like tests/tools/terminal/test_session_factory.py.

else, it will raise ModuleNotFoundError: No module named 'fcntl'
https://github.com/OpenHands/OpenHands/issues/11629

ryanhoangt · 2025-11-17T10:54:12Z

openhands-tools/openhands/tools/terminal/terminal/factory.py

            logger.info("Using forced TmuxTerminal")
            terminal = TmuxTerminal(work_dir, username)
            return TerminalSession(terminal, no_change_timeout_seconds)
        elif terminal_type == "subprocess":


Since SubprocessTerminal depends on fcntl which isn't available on Windows, should we add a if platform.system() == "Windows": check here and raise RuntimeError?

line 86 will run only when terminal_type== "subprocess" is specified and None is the default value for that.

SmartManoj · 2025-11-17T12:55:34Z

I saw it first attempted to run a bash command

Does this tool description need to be updated?

software-agent-sdk/openhands-tools/openhands/tools/terminal/definition.py

Line 191 in ec8c806

    
           TOOL_DESCRIPTION = """Execute a bash command in the terminal within a persistent shell session.

xingyaoww · 2025-11-17T15:10:44Z

@SmartManoj

Does this tool description need to be updated?

Yes we should either phrase it so the description is general enough, OR we should implement a switch (i.e., only change the description if windows is selected as the terminal implementation)

Introduces TOOL_DESCRIPTION_FOR_WINDOWS for PowerShell commands and updates TerminalTool to select the appropriate description based on the platform. This improves clarity and guidance for users on both Unix and Windows systems.

SmartManoj · 2025-11-18T08:33:03Z

openhands-tools/openhands/tools/terminal/terminal/windows_terminal.py

ryanhoangt

This is great and LGTM, thank you!

xingyaoww

@ryanhoangt, can we potentially get LLM-back integration tests running on Windows as well? Maybe in a subsequent PR

pyproject.toml

xingyaoww · 2025-11-19T18:42:12Z

.github/workflows/tests.yml

+                    --cov-report=term-missing `
+                    --cov-fail-under=0 `
+                    --cov-config=pyproject.toml `
+                    tests/tools/terminal/test_windows_terminal.py


Can we run all the tests here instead of just the terminal one?

Ideally we should check all things are working on Windows (maybe just skip subprocess/tmux terminals).

#1209 - Follow up

Deleted the [tool.coverage.paths] section, which specified source paths for coverage analysis. This simplifies the configuration and may indicate a change in how coverage is managed or reported.

SmartManoj · 2025-11-20T04:49:52Z

Pre-requisite #1014?

enyst · 2025-11-20T10:07:05Z

Pre-requisite #1014?

Are the failing tests browser tests? That PR only refers to the browser tool.

SmartManoj · 2025-11-20T10:09:25Z

enyst · 2025-11-20T11:16:50Z

When I look at the failed github action, I see 69 tests failed:

...
ERROR tests/tools/file_editor/test_basic_operations.py::test_view_operation_truncation - PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpsqa3coyk'
ERROR tests/tools/file_editor/test_error_handling.py::test_str_replace_error_handling - PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmphasdsv36'
ERROR tests/tools/file_editor/test_error_handling.py::test_view_range_validation - PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmp1_4_wj1l'
ERROR tests/tools/file_editor/test_error_handling.py::test_insert_validation - PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmp8005bwo_'
ERROR tests/tools/file_editor/test_error_handling.py::test_undo_validation - PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpcgu4pil1'
= 69 failed, 314 passed, 24 skipped, 36 warnings, 32 errors in 192.79s (0:03:12) =
Windows tools coverage file prepared for upload
Error: Process completed with exit code 1.

Seems like file editor tests are failing, not just browser?

SmartManoj · 2025-11-20T11:39:07Z

enyst · 2025-11-20T12:24:29Z

tests/tools/file_editor/conftest.py

+        yield file_path
+        # On Windows, files may be locked briefly after handles are closed.
+        # This is due to Windows file system behavior where file handles may not be
+        # immediately released, opportunistic locks (oplocks), or background processes


Could this affect other behaviors too, not just current tests? 🤔

Deleted the [tool.coverage.report] section and its ignore_errors setting from pyproject.toml, likely to simplify coverage configuration or rely on defaults.

Moved the Unix and Windows terminal tool descriptions to separate template files and updated the code to load them dynamically. This improves maintainability and allows easier updates to the descriptions without modifying the Python source.

Renamed unix and windows terminal description templates from .txt to .j2 for consistency with Jinja2 template naming. Updated references in definition.py and included the new template files in the agent-server.spec data collection.

blacksmith-sh · 2025-11-26T13:00:41Z

[Automatic Post]: It has been a while since there was any activity on this PR. @SmartManoj, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

blacksmith-sh · 2025-12-01T13:03:11Z

[Automatic Post]: It has been a while since there was any activity on this PR. @SmartManoj, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

dismiss

xingyaoww

Actually @SmartManoj could we combine #1209 with this PR - so we can be sure the windows terminal is working as expected before we merge it? Thanks!

SmartManoj · 2025-12-01T16:05:33Z

Pre-requisite #1014?

@xingyaoww

enyst · 2025-12-01T17:23:32Z

Pre-requisite #1014?

I'm happy to unblock that issue. I think there is a bit of a discussion in the related issue on how exactly we design the code.

Add Windows terminal backend support

6789d81

Introduces WindowsTerminal for PowerShell-based command execution on Windows systems. Updates factory and init logic to conditionally import and use platform-specific terminal backends, ensuring compatibility across Windows and Unix-like platforms.

xingyaoww requested a review from ryanhoangt November 4, 2025 15:42

xingyaoww reviewed Nov 4, 2025

View reviewed changes

SmartManoj added 4 commits November 5, 2025 12:19

ruff

a1f6319

Remove platform check and always import fcntl and pty for pyright

4e064b0

Simplifies the code by unconditionally importing fcntl and pty, removing the platform-specific logic and dummy assignments for Windows. This module is intended for Unix-like systems only.

pyright

baa383e

Merge branch 'main' into terminal

4efe972

SmartManoj mentioned this pull request Nov 5, 2025

Rename execute_bash to terminal & rename BashTool to TerminalTool #1033

Merged

ruff

7939e98

SmartManoj added 4 commits November 6, 2025 08:05

Remove demo script

a7b35dd

Deleted scripts/demo.py as it is no longer needed. The demo script previously demonstrated basic usage of the OpenHands SDK and agent setup.

Merge branch 'main' into terminal

0eb8eea

Remove stray character from import statement

4fefd40

Eliminated an erroneous 'n' character from the import block in terminal_session.py to clean up the code and prevent potential syntax errors.

Update assertions to use obs.text in Windows Terminal tests

d252b38

Changed assertions in test_windows_terminal_close_and_reopen to check 'obs.text' instead of 'obs.output' for command results. This aligns the test with the updated observation object structure.

SmartManoj added 3 commits November 7, 2025 03:19

Merge branch 'main' into terminal

a2123bb

Merge branch 'main' into terminal

d0ca912

SmartManoj requested a review from xingyaoww November 15, 2025 11:12

ryanhoangt reviewed Nov 17, 2025

View reviewed changes

Add Windows-specific terminal tool description

78791dd

Introduces TOOL_DESCRIPTION_FOR_WINDOWS for PowerShell commands and updates TerminalTool to select the appropriate description based on the platform. This improves clarity and guidance for users on both Unix and Windows systems.

SmartManoj force-pushed the terminal branch from f6f91b7 to 78791dd Compare November 18, 2025 01:51

SmartManoj added 2 commits November 18, 2025 11:21

format

bd5e3b9

Merge branch 'main' into terminal

d0002c4

ryanhoangt self-requested a review November 18, 2025 14:36

lint

2101caf

ryanhoangt reviewed Nov 19, 2025

View reviewed changes

openhands-tools/openhands/tools/terminal/terminal/windows_terminal.py Show resolved Hide resolved

add a few tests

54a8749

ryanhoangt approved these changes Nov 19, 2025

View reviewed changes

xingyaoww previously requested changes Nov 19, 2025

View reviewed changes

SmartManoj added 2 commits November 20, 2025 08:15

Remove coverage paths configuration from pyproject.toml

3f2dc44

Deleted the [tool.coverage.paths] section, which specified source paths for coverage analysis. This simplifies the configuration and may indicate a change in how coverage is managed or reported.

Merge branch 'main' into terminal

c28d605

enyst reviewed Nov 20, 2025

View reviewed changes

Remove coverage report config from pyproject.toml

c4869a6

Deleted the [tool.coverage.report] section and its ignore_errors setting from pyproject.toml, likely to simplify coverage configuration or rely on defaults.

SmartManoj force-pushed the terminal branch from 4a37779 to c4869a6 Compare November 20, 2025 12:26

SmartManoj mentioned this pull request Nov 20, 2025

Expand Windows tools test coverage in CI workflow #1209

Open

Merge branch 'main' into terminal

c40789b

enyst mentioned this pull request Nov 20, 2025

Proposal: separate Windows code and dogfooding #1210

Open

SmartManoj added 3 commits November 21, 2025 08:05

Merge branch 'main' into terminal

b2416e0

Rename terminal description templates to .j2 extension

805630e

Renamed unix and windows terminal description templates from .txt to .j2 for consistency with Jinja2 template naming. Updated references in definition.py and included the new template files in the agent-server.spec data collection.

SmartManoj requested a review from xingyaoww November 21, 2025 06:18

xingyaoww requested changes Dec 1, 2025

View reviewed changes

Merge branch 'main' into terminal

2626ec7

ryanhoangt mentioned this pull request Dec 4, 2025

Add Windows browser path checks to Chromium detection #1014

Open

Add Windows terminal backend support #1012

Are you sure you want to change the base?

Add Windows terminal backend support #1012

Uh oh!

Conversation

SmartManoj commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xingyaoww left a comment

Choose a reason for hiding this comment

Uh oh!

SmartManoj commented Nov 5, 2025

Uh oh!

xingyaoww commented Nov 6, 2025

Uh oh!

blacksmith-sh bot commented Nov 10, 2025

Uh oh!

ryanhoangt left a comment

Choose a reason for hiding this comment

Uh oh!

ryanhoangt Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

SmartManoj Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

ryanhoangt Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

SmartManoj Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

SmartManoj commented Nov 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xingyaoww commented Nov 17, 2025

Uh oh!

SmartManoj commented Nov 18, 2025

Uh oh!

Uh oh!

ryanhoangt left a comment

Choose a reason for hiding this comment

Uh oh!

xingyaoww left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xingyaoww Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

SmartManoj Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

SmartManoj commented Nov 20, 2025

Uh oh!

enyst commented Nov 20, 2025

Uh oh!

SmartManoj commented Nov 20, 2025

Uh oh!

enyst commented Nov 20, 2025

Uh oh!

SmartManoj commented Nov 20, 2025

Uh oh!

enyst Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

blacksmith-sh bot commented Nov 26, 2025

Uh oh!

blacksmith-sh bot commented Dec 1, 2025

Uh oh!

xingyaoww left a comment

Choose a reason for hiding this comment

Uh oh!

SmartManoj commented Dec 1, 2025

Uh oh!

enyst commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

SmartManoj commented Nov 4, 2025 •

edited

Loading

SmartManoj commented Nov 17, 2025 •

edited

Loading